C#: Fewer alerts in cs/useless-if-statement.#18935
Conversation
|
DCA looks good. |
There was a problem hiding this comment.
PR Overview
This PR adjusts the behavior of the cs/useless-if-statement query to reduce alerts when an empty if-statement contains a comment. It updates test cases to reflect the new behavior and updates the documentation to clarify that if a comment is present, the statement is not flagged.
Reviewed Changes
| File | Description |
|---|---|
| csharp/ql/test/query-tests/Useless Code/FutileConditional/FutileConditional.cs | Adds test cases that demonstrate when alerts should and should not be raised |
| csharp/ql/src/change-notes/2025-03-05-useless-if-statement.md | Updates the change notes to document the new behavior for commented if-blocks |
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more
| if (s.Length > 2) // GOOD: because of else-branch | ||
| { | ||
| } |
There was a problem hiding this comment.
Unrelated to your change, I find it odd to consider this okay.
There was a problem hiding this comment.
Yes, I agree. Maybe an idea for a quality query:
Create a new quality query which states that the condition should be inverted (we can also provide good QHelp for that - and I suspect an LLM will be good at figuring out how the fix should look like).
There was a problem hiding this comment.
Hmm there is already a button for that in VSCode, so it might not be relevant (but you will have to actively click that).
It is not an un-common pattern to write something like
Due to the above our query might seem a bit to noisy. In this PR we remove the alerts, if the empty if-block contains a comment.
We still catch the following examples (where the first one is the most important as it is most likely unintentional).